Skip to content

New routine to traverse all nodes of a JSON structure. #144

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 3 commits into from
Aug 4, 2015
Merged

New routine to traverse all nodes of a JSON structure. #144

merged 3 commits into from
Aug 4, 2015

Conversation

jacobwilliams
Copy link
Owner

See #140

@codecov-io
Copy link

Current coverage is 90.82%

Merging #144 into master will increase coverage by +0.08% as of 8dc7fe6

@@            master    #144   diff @@
======================================
  Files            1       1       
  Stmts         1468    1481    +13
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit           1332    1345    +13
  Partial          0       0       
  Missed         136     136

Review entire Coverage Diff

Uncovered Suggestions

  1. +0.27% via src/json_module.F90#6292...6295
  2. +0.27% via src/json_module.F90#6050...6053
  3. +0.27% via src/json_module.F90#3966...3969
  4. See 7 more...

Powered by Codecov

@codecov-io
Copy link

Current coverage is 90.82%

Merging traverse into master will increase coverage by +0.08% as of 8200541

@@            master   traverse   diff @@
=========================================
  Files            1          1       
  Stmts         1468       1481    +13
  Branches         0          0       
  Methods          0          0       
=========================================
+ Hit           1332       1345    +13
  Partial          0          0       
  Missed         136        136

Review entire Coverage Diff

Uncovered Suggestions

  1. +0.27% via src/json_module.F90#6292...6295
  2. +0.27% via src/json_module.F90#6050...6053
  3. +0.27% via src/json_module.F90#3966...3969
  4. See 7 more...

Powered by Codecov

@zbeekman
Copy link
Contributor

zbeekman commented Aug 3, 2015

Looks good to me. One comment: would you mind changing the output so only json_print (and other stuff that creates valid JSON structures) is writing to stdout, and anything informational or relating to status, warning and error messages is written to stderr? This is the convention for most/all of the other tests, with the goal being to eventually validate their outputs with jsonlint in the CI test suite. The only thing, as far as I remember, that is preventing this is a few tests output multiple valid JSON objects which the linter we were using flagged as problematic.

zbeekman
zbeekman reviewed Aug 3, 2015
View reviewed changes
src/tests/jf_test_14.f90
end if

if (error_cnt==0) then
write(output_unit,'(A)') ''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d prefer everything other than json_print to write to stderr so that one day I can finish implementing the output validation with a JSON linter.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's fine. Will do that later today.

jacobwilliams added a commit that referenced this pull request Aug 4, 2015
@jacobwilliams
Merge pull request #144 from jacobwilliams/traverse
331bde2 
New routine to traverse all nodes of a JSON structure. Fixes #140
@jacobwilliams jacobwilliams merged commit 331bde2 into master Aug 4, 2015
@jacobwilliams jacobwilliams deleted the traverse branch August 4, 2015 00:05
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jacobwilliams @codecov-io @zbeekman